feat(room-worker): create-room origin-site MV fix per spec#169
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR implements a fix for newly created federated rooms being invisible in search and spotlight. The core changes add local INBOX ChangesOrigin-Site Member_Added Publish for Room Creation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inbox-worker/handler.go (1)
311-316:⚠️ Potential issue | 🟠 Major | ⚡ Quick winContinue through duplicate-key replays before returning.
The duplicate-key branch on Line 313 exits before the new local
member_addedpublish runs. If a prior delivery inserted the subscriptions and then died before publishing, the redelivery will hit this path and leave the remote MV stale indefinitely. Treat duplicate-key as idempotent and fall through to the publish block; theNats-Msg-Idalready makes the replay safe.Suggested fix
if err := h.store.BulkCreateSubscriptions(ctx, subs); err != nil { - if mongo.IsDuplicateKeyError(err) { - return nil - } - return fmt.Errorf("bulk create subs: %w", err) + if !mongo.IsDuplicateKeyError(err) { + return fmt.Errorf("bulk create subs: %w", err) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inbox-worker/handler.go` around lines 311 - 316, The BulkCreateSubscriptions call currently returns early when mongo.IsDuplicateKeyError(err) is true, preventing the subsequent publish of the local "member_added" event; change the error branch in the h.store.BulkCreateSubscriptions handling so duplicate-key errors are treated as idempotent (do not return) and execution falls through to the publish logic (only return fmt.Errorf("bulk create subs: %w", err) for non-duplicate errors). Specifically update the block around h.store.BulkCreateSubscriptions and mongo.IsDuplicateKeyError to log/ignore duplicate-key and continue to the member_added publish (replay is safe due to Nats-Msg-Id).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@inbox-worker/handler.go`:
- Around line 311-316: The BulkCreateSubscriptions call currently returns early
when mongo.IsDuplicateKeyError(err) is true, preventing the subsequent publish
of the local "member_added" event; change the error branch in the
h.store.BulkCreateSubscriptions handling so duplicate-key errors are treated as
idempotent (do not return) and execution falls through to the publish logic
(only return fmt.Errorf("bulk create subs: %w", err) for non-duplicate errors).
Specifically update the block around h.store.BulkCreateSubscriptions and
mongo.IsDuplicateKeyError to log/ignore duplicate-key and continue to the
member_added publish (replay is safe due to Nats-Msg-Id).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53e4b377-c43f-403a-9bc3-7a450f1e5c1f
📒 Files selected for processing (9)
docs/superpowers/specs/2026-05-11-create-room-origin-site-mv-fix-design.mdinbox-worker/handler.goinbox-worker/handler_test.goinbox-worker/integration_test.goinbox-worker/main.gopkg/natsutil/request_id.goroom-worker/handler.goroom-worker/handler_test.goroom-worker/integration_test.go
Joey0538
force-pushed
the
claude/spec-create-room-inbox-publish
branch
2 times, most recently
from
780807a to
a47ab60
Compare
Joey0538
changed the title
Joey0538
force-pushed
the
claude/spec-create-room-inbox-publish
branch
from
a47ab60 to
859d137
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
room-worker/handler.go (2)
1183-1191: ⚡ Quick winConsider explicitly setting
HistorySharedSince: nilfor clarity.Same issue as the local INBOX publish: the
MemberAddEventat lines 1183-1191 omitsHistorySharedSince, relying on the zero value. For consistency withprocessAddMembers(line 813), explicitly settingHistorySharedSince: nildocuments the intent and aligns with the established pattern.♻️ Proposed fix
memberEvt := model.MemberAddEvent{ Type: model.OutboxMemberAdded, RoomID: room.ID, RoomName: room.Name, Accounts: accounts, SiteID: room.SiteID, JoinedAt: req.Timestamp, + HistorySharedSince: nil, Timestamp: now.UnixMilli(), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@room-worker/handler.go` around lines 1183 - 1191, The MemberAddEvent construction used for the outbox publish (the MemberAddEvent literal at the top-level in handler.go) omits HistorySharedSince; to match processAddMembers and make intent explicit, add HistorySharedSince: nil to the MemberAddEvent literal (the struct created with Type: model.OutboxMemberAdded, RoomID: room.ID, etc.) so the field is explicitly set to nil rather than left to the zero value.
1118-1126: ⚡ Quick winConsider explicitly setting
HistorySharedSince: nilfor clarity.The
MemberAddEventstruct at line 1118-1126 omits theHistorySharedSincefield, relying on the zero value (nil for pointer types). While correct per the spec ("Always nil — no prior history at create time"), the parallel code inprocessAddMembers(line 744) explicitly assignsHistorySharedSince: historySharedSincefor clarity.Explicitly setting
HistorySharedSince: nilhere improves consistency and makes the intent self-documenting.♻️ Proposed fix
inner := model.MemberAddEvent{ Type: model.OutboxMemberAdded, RoomID: room.ID, RoomName: room.Name, Accounts: accounts, SiteID: room.SiteID, JoinedAt: req.Timestamp, + HistorySharedSince: nil, Timestamp: now.UnixMilli(), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@room-worker/handler.go` around lines 1118 - 1126, The MemberAddEvent construction (variable inner) omits HistorySharedSince; explicitly set HistorySharedSince: nil to make the "no prior history" intent explicit and consistent with processAddMembers which sets HistorySharedSince there; update the inner = model.MemberAddEvent{ ... } initializer to include HistorySharedSince: nil alongside the other fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@room-worker/handler.go`:
- Around line 1183-1191: The MemberAddEvent construction used for the outbox
publish (the MemberAddEvent literal at the top-level in handler.go) omits
HistorySharedSince; to match processAddMembers and make intent explicit, add
HistorySharedSince: nil to the MemberAddEvent literal (the struct created with
Type: model.OutboxMemberAdded, RoomID: room.ID, etc.) so the field is explicitly
set to nil rather than left to the zero value.
- Around line 1118-1126: The MemberAddEvent construction (variable inner) omits
HistorySharedSince; explicitly set HistorySharedSince: nil to make the "no prior
history" intent explicit and consistent with processAddMembers which sets
HistorySharedSince there; update the inner = model.MemberAddEvent{ ... }
initializer to include HistorySharedSince: nil alongside the other fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9949322c-2d2e-4945-ab14-c3c42d73dc7a
📒 Files selected for processing (5)
docs/superpowers/specs/2026-05-11-create-room-origin-site-mv-fix-design.mdpkg/natsutil/request_id.goroom-worker/handler.goroom-worker/handler_test.goroom-worker/integration_test.go
✅ Files skipped from review due to trivial changes (1)
- room-worker/integration_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/natsutil/request_id.go
- room-worker/handler_test.go
Joey0538
force-pushed
the
claude/spec-create-room-inbox-publish
branch
from
859d137 to
cf54899
Compare
Sibling fix to PR #145 (federated-room MV update for add/remove): apply the same local-INBOX publish pattern to the room-creation path so freshly-created rooms appear in user-room and spotlight indexes immediately, not on the next add/remove. Two narrow additions: one publish at the end of room-worker's finishCreateRoom (origin site) and a symmetric one at the end of inbox-worker's handleRoomCreated (federated remote sites). Wire format byte-for-byte matches PR #145 so search-sync-worker decodes both identically. Forward-only rollout per agreement; no backfill tool. https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Sibling fix to PR #145 (federated-room MV update for add/remove). PR #145 closed the origin-site MV gap for member.add / member.remove; this PR applies the same local-INBOX + cross-site OUTBOX pattern to the room-creation path so freshly-created rooms appear in user-room-{site} and spotlight-{site} ES indexes immediately, not on the next add/remove operation. room-worker.finishCreateRoom now emits two new publishes: 1. Local origin-site INBOX: chat.inbox.{origin}.member_added carrying every account in subs[] (creator + every auto-enrolled member). Drives the origin site's search-sync-worker MV update. 2. Cross-site OUTBOX per remote site: outbox.{origin}.to.{remote}. member_added carrying only the remote-site accounts (per-dest split). Reuses the existing federation lane PR #145 established for add-members: SubjectTransform rewrites it to chat.inbox.{remote}. aggregate.member_added, which the remote site's search-sync-worker already consumes. No new inbox-worker code, no new event types, no new stream config — just one more publish on a path that already exists. Wire format byte-for-byte identical to PR #145 so parseMemberEvent decodes all member_added events the same way regardless of which path they take. Also: lift outboxDedupID from room-worker's private helper to natsutil.OutboxDedupID — used in 9 call sites on this branch, removes the copy I would have introduced if inbox-worker had needed to publish too. Tests: 3 new unit tests in room-worker (DM local INBOX, channel local INBOX, channel cross-site OUTBOX member_added). Forward-only rollout per spec; no backfill tool. Spec: docs/superpowers/specs/2026-05-11-create-room-origin-site-mv-fix-design.md https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
DurableConsumerDefaults from PR #168 hardcoded DeliverPolicy=New for every durable JetStream consumer. That's wrong for our workload: - search-sync-worker's user-room-sync and spotlight-sync rebuild ES indexes from the INBOX stream. With DeliverNew, a fresh durable (new deploy, new site, deleted-and-recreated durable) starts at HEAD and the MV is permanently missing every historical event. - inbox-worker handles cross-site federated arrivals. With DeliverNew, any catch-up after a stream-side gap is lost — the local state diverges from the remote OUTBOX silently. - broadcast-worker / message-worker / notification-worker / message-gatekeeper / room-worker: same risk if a durable ever needs to be recreated from scratch. Flip the project-wide invariant to DeliverAll. For streams with no historical data (steady-state new sites) All and New are equivalent; for any catch-up or rebuild scenario All is the only correct choice. DeliverPolicy is only honored at consumer creation, so existing durables in prod are unaffected — this only changes behavior for new consumers (new deploys, new sites, durables deleted and recreated). Updates the doc comment on DurableConsumerDefaults and the eight consumer_config_test.go invariant assertions. https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
force-pushed
the
claude/spec-create-room-inbox-publish
branch
from
cf54899 to
4aa9606
Compare
mliu33
reviewed
May 11, 2026
| return jetstream.ConsumerConfig{ | ||
| AckPolicy: jetstream.AckExplicitPolicy, | ||
| DeliverPolicy: jetstream.DeliverNewPolicy, | ||
| DeliverPolicy: jetstream.DeliverAllPolicy, |
vjauhari-work
pushed a commit
that referenced
this pull request
May 11, 2026
Brings the PR up to date with main. Two PRs landed since this branch forked: - #167 (read-receipt RPC) added a `ListReadReceipts` method to `room-service` and reworked `NewHandler` to take a `MessageReader` argument. Conflicts in `room-service/store.go`, `room-service/store_mongo.go`, `room-service/handler_test.go`, and `room-service/mock_store_test.go` are all additive — both `Set/Rotate` / `CountOrgOnlySubs` and the new read-receipt methods are kept. Test handlers updated to use main's 9-arg `NewHandler` signature. - #169 (room-worker MV fix) added new tests at the bottom of `room-worker/handler_test.go`. Combined with my key-path tests by keeping both sets; new tests wire up `testKeyStore`/`testKeySender` since VALKEY is now a hard runtime dependency. Also regenerated `room-service/mock_store_test.go` via `make generate` to cover both branches' added store methods. https://claude.ai/code/session_013m3j9nudXZz2j29kopFQ51
This was referenced May 12, 2026
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 12, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 18, 2026
Drop the redundant outbox.{origin}.to.{remote}.room_created event in
favor of the existing outbox.{origin}.to.{remote}.member_added event
(added in PR #169) doing double duty: drive sub creation in inbox-worker
AND MV update in search-sync-worker, mirroring the add-members path
which already works that way since PR #145.
Extend MemberAddEvent with RoomType + RequesterAccount so
inbox-worker.handleMemberAdded can build correctly-shaped DM/botDM subs
via the existing helpers (subscriptionName / rolesForType /
subscriptionIsSubscribed) instead of needing a separate handleRoomCreated
path.
Full removal of room_created event, model, handler, and tests. Incidental
benefit: heals a latent search-sync-worker bug where the spotlight ES
doc's roomType field has been empty since PR #145 because today's
MemberAddEvent wire format doesn't carry RoomType.
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
pushed a commit
that referenced
this pull request
May 18, 2026
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
added a commit
that referenced
this pull request
May 18, 2026
* docs(spec): consolidate cross-site room-creation federation event
Drop the redundant outbox.{origin}.to.{remote}.room_created event in
favor of the existing outbox.{origin}.to.{remote}.member_added event
(added in PR #169) doing double duty: drive sub creation in inbox-worker
AND MV update in search-sync-worker, mirroring the add-members path
which already works that way since PR #145.
Extend MemberAddEvent with RoomType + RequesterAccount so
inbox-worker.handleMemberAdded can build correctly-shaped DM/botDM subs
via the existing helpers (subscriptionName / rolesForType /
subscriptionIsSubscribed) instead of needing a separate handleRoomCreated
path.
Full removal of room_created event, model, handler, and tests. Incidental
benefit: heals a latent search-sync-worker bug where the spotlight ES
doc's roomType field has been empty since PR #145 because today's
MemberAddEvent wire format doesn't carry RoomType.
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
* docs(spec): address CodeRabbit review on federation consolidation
- Line 160: "consts" -> "constants" (style)
- Line 208: "sub shape" -> "sub-shape" (hyphenation)
- Rollout section: rewrite to honestly document the
no-fully-safe-single-PR-deploy-order issue CodeRabbit flagged.
Walks both deploy orders showing the malformed-DM window each
produces. Presents three options (A: ship as-is, B: 2-PR split,
C: single PR + follow-up cleanup) with a recommendation for
option C. Marks this as an open question requiring user input
before implementation begins.
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
* docs(spec): simplify rollout to single-PR per pre-prod context
User confirmed cross-site federation is not yet integrated end-to-end,
so the theoretical mixed-version DM-sub-malformation window has no
real-world incidence today. Reverting the spec to option (A):
single PR ships both publisher and consumer changes; deploy order
(room-worker first) is defensive rather than strictly required.
Trims the option-discussion text and rolls the deploy-window risk into
the Risks section with explicit acknowledgment that it's theoretical
under current operational reality.
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
* feat(model,room-worker,inbox-worker): consolidate room-create federation
Single cross-site event for room creation: outbox.{origin}.to.{remote}.
member_added does double duty — drives sub creation in inbox-worker
(with correct DM/botDM/channel shapes) AND MV update in
search-sync-worker. Drops the redundant room_created event entirely.
Schema (pkg/model/event.go):
- MemberAddEvent gains RoomType + RequesterAccount (both omitempty).
- Delete RoomCreatedOutbox struct.
- Delete OutboxTypeRoomCreated constant.
- MessageTypeRoomCreated stays — distinct system-message-type constant
used by room-worker's publishChannelSysMessages, unrelated to
federation.
Consumer (inbox-worker/handler.go):
- handleMemberAdded dispatches on event.RoomType. Empty RoomType
defaults to RoomTypeChannel for backward-compat with pre-deploy
publishers that didn't set the field.
- subscriptionName / subscriptionIsSubscribed helpers refactored to
take primitives (roomType, roomName, requesterAccount, *user)
instead of *RoomCreatedOutbox, so handleMemberAdded can call them.
- Duplicate-key BulkCreateSubscriptions errors swallowed (replay
after a crashed prior delivery is idempotent — matches PR #169 fix).
- handleRoomCreated function deleted.
- case model.MessageTypeRoomCreated arm in HandleEvent switch deleted.
Publisher (room-worker/handler.go):
- finishCreateRoom: delete the per-remote-site room_created OUTBOX
publish. Cross-site member_added publish now carries RoomType +
RequesterAccount.
- finishCreateRoom local INBOX publish: same fields populated for
consistency (search-sync-worker reads them).
- processAddMembers: populate RoomType + RequesterAccount on all
three member_added publishes (UI fan-out, local INBOX, cross-site
OUTBOX). Channels-only path, but consistent shape avoids surprises.
- publishSyncDMOutbox: switch from room_created to member_added with
the full new schema.
Tests:
- inbox-worker/handler_test.go: replace 5 TestHandleRoomCreated* tests
with TestHandleMemberAdded_DM/BotDM/Channel/EmptyRoomType/
DuplicateKey cases. Helpers refactored to match new signatures.
- inbox-worker/integration_test.go: replace 2 room_created integration
tests with member_added equivalents going through HandleEvent.
- room-worker/handler_test.go + integration_test.go: assertions on
cross-site outboxes now look for OutboxMemberAdded subjects with
full RoomType + RequesterAccount payload.
Incidental fix: search-sync-worker.spotlight.go has been writing an
empty `roomType` field to the spotlight ES doc since PR #145 because
MemberAddEvent's wire format didn't carry RoomType. Once room-worker
starts populating RoomType, the spotlight doc gets correct roomType
for the first time. No code change in search-sync-worker; existing
TestSpotlightCollection_BuildAction_MemberAdded asserts the correct
value.
Spec: docs/superpowers/specs/2026-05-12-consolidate-room-create-federation-design.md
https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
---------
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sibling fix to PR #145 (federated-room MV update for add/remove). Apply the same publish pattern to the room-creation path so freshly-created rooms appear in
user-room-{site}andspotlight-{site}ES indexes immediately, not on the next add/remove operation.Spec:
docs/superpowers/specs/2026-05-11-create-room-origin-site-mv-fix-design.mdThe bug, in one sentence
room-worker.finishCreateRoomwrites the auto-enrolledSubscriptionrows but never publishes amember_addedevent — sosearch-sync-worker's MV indexes never see the new room until later churn.User-visible symptoms
user-room-{site}doc reports them as not subscribed.Fix
Two publishes added to
room-worker.finishCreateRoom. Both wire-format-compatible with PR #145.chat.inbox.{origin}.member_addedsearch-sync-worker→user-room-{origin}+spotlight-{origin}outbox.{origin}.to.{remote}.member_addedsearch-sync-workervia the existing aggregate lane →user-room-{remote}+spotlight-{remote}inbox-workeris intentionally untouched. It continues to consumeaggregate.room_createdfor sub mirroring (its current job). Earlier draft revisions tried to makeinbox-workerrepublish a localmember_addedon the remote side after creating the subs — that worked but added a second hop, grew theHandlersurface, and broke on duplicate-key replays. Moving the cross-sitemember_addedintoroom-worker(mirroringprocessAddMembers's federation pattern exactly) meanssearch-sync-workeron the remote site gets the event via the same aggregate-lane path that already works for add-members.Drive-by
Lifted
outboxDedupIDfromroom-worker's private helper intonatsutil.OutboxDedupID— pure logic, used in 9 call sites, removes a copy that the original draft would have introduced.What this does NOT change
pkg/subject,pkg/stream,pkg/model— no new types/subjects.inbox-worker— untouched.search-sync-worker,message-worker,broadcast-worker,history-service— untouched.Tests
Unit tests only:
room-worker/handler_test.go: DM origin-local INBOX, channel origin-local INBOX, channel cross-site OUTBOXmember_added.TestProcessCreateRoomChannel_OutboxPerRemoteSite,TestProcessCreateRoomDM_OutboxToCounterpartSite).Rollout
Forward-only per the spec. Pre-fix rooms catch up on next churn or stay missing — acceptable. Both new publishes are additive on subjects/lanes that already exist (PR #145).
Test plan
make lintcleanmake testclean (full repo)go vet -tags integration ./...cleanuser-room-{site}ES index and confirm the creator/recipient/initial-members appear with the new roomIDFiles
docs/superpowers/specs/2026-05-11-create-room-origin-site-mv-fix-design.md(NEW)pkg/natsutil/request_id.go— newOutboxDedupIDhelperroom-worker/handler.go—finishCreateRoomadds 2 publishes; existing 9outboxDedupIDcall sites migrated tonatsutil.OutboxDedupIDroom-worker/handler_test.go— 3 new unit testsroom-worker/integration_test.go— 2 existing tests updated for the new dual-outbox shapeDesign history note
Earlier revisions of this PR had
inbox-worker.handleRoomCreatedrepublish a localmember_addedafter creating the subs on the remote side. CodeRabbit caught a latent duplicate-key bug in that path (replay after a crashed prior delivery would silently skip the publish). The current design moves the cross-sitemember_addedpublish intoroom-workerinstead, where it mirrorsprocessAddMembersexactly and the entire federation lane is reused —inbox-workerdoesn't need to grow any new surface.https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp